Skip to content

No queue arguments needed on SLURM for most clusters.#19

Closed
bw4sz wants to merge 3 commits into
dask:masterfrom
bw4sz:no_queue
Closed

No queue arguments needed on SLURM for most clusters.#19
bw4sz wants to merge 3 commits into
dask:masterfrom
bw4sz:no_queue

Conversation

@bw4sz

@bw4sz bw4sz commented Mar 21, 2018

Copy link
Copy Markdown

Removed queue from default arguments. The default was empty, which caused the following arguments to be poorly parsed by SLURM. The university clusters I have worked with do not use the -p flag. Of course, it may vary by cluster. This reverts #18, which probably wasn't needed.

Closes #15

@bw4sz

bw4sz commented Mar 21, 2018

Copy link
Copy Markdown
Author

Confirmed works on University of Florida HPC environment.

(pangeo) [b.weinstein@gator3 dask-jobqueue]$ python test_slurm.py 
(pangeo) [b.weinstein@gator3 dask-jobqueue]$ squeue -u b.weinstein
             JOBID PARTITION     NAME     USER ST       TIME  NODES NODELIST(REASON)
          18337278 hpg2-comp     dask b.weinst  R       0:14      2 c31a-s[4,21]
          18337279 hpg2-comp     dask b.weinst  R       0:14      1 c22b-s42
          18337280 hpg2-comp     dask b.weinst  R       0:14      2 c22a-s36,c22b-s40
(pangeo) [b.weinstein@gator3 dask-jobqueue]$ cat test_slurm.py 

from dask_jobqueue import SLURMCluster

cluster = SLURMCluster(project='ewhite')
cluster.start_workers(3)

@jhamman

jhamman commented Mar 23, 2018

Copy link
Copy Markdown
Member

Let's put this on hold until after #10 gets merged. I think there will need to be some minor modifications to the SLURMCluster but they should come after #10.

@guillaumeeb guillaumeeb mentioned this pull request Mar 23, 2018
Closed
@guillaumeeb

guillaumeeb commented Mar 23, 2018

Copy link
Copy Markdown
Member

Yup, for consistency, we will have to refactor SLURMCluster as mentionned in #22. As for this pull request, I'm not entirely sure removing queue keyword is a good thing. Don't know Slurm, but I believe being able to specify a queue is a good thing.

After #10 is merged, we may put the queue keyword as None by default, and by building the job_script dynamically, we could avoid add the -p option being positioned in the script.

@guillaumeeb guillaumeeb mentioned this pull request Mar 23, 2018
Closed
@jhamman jhamman mentioned this pull request Mar 23, 2018
Closed
@guillaumeeb guillaumeeb mentioned this pull request Mar 27, 2018
Merged
@jhamman jhamman closed this in #25 Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bw4sz @jhamman @guillaumeeb